This is looking good.

Some more updates.

• The findRaw and aggregateRaw methods have been updated to sort before projecting. This should ensure that a user can sort by a property that is not projected.
• The countDocuments method no longer returns 0 if service.options.paginate = undefined. The user shouldn't have to have the paginate option if they want to count documents. It doesn't rely on pagination options anyway; it counts all the documents by design. It can also now handle counting with the aggregation framework.
• The _find method has been optimized to better handle when to count and when not to. Specifically, there have been logical branches added for filters.$limit === 0 then just count and don't find, if params.paginate === false just find and don't count.

See further comments in the _find method for some more questions/context.

Another note about counting and aggregation. We could finesse the pipleline with a $count stage. We would coerce the result into a "page" with { total: 100, data: [ ... ] } right out of the pipeline instead of just an array of data. This would alleviate the need to do two separate count and find operations in that case. But, it seems complicated and we might bash the user's custom pipeline stages.

@marshallswain I believe I am done with this PR. I am currently testing it in a project locally and everything seems to be working as expected!

This PR is complete. @marshallswain and @daffl

I'm letting @marshallswain have a look at this since he did a lot of that work before. Any idea what's going on with the tests?

I am not sure what you mean about the tests. They seem to be working fine for me.

I think it's a good idea to let Marshall look at this too. I do believe this is a major version bump for two reasons. The adapter previously used params.mongo to determine which find method to use. If params.mongo was present it used Model.find and otherwise used Model.aggregate. The adapter now uses params.pipeline to make this decision. If present, it uses Model.aggregate and uses Model.find otherwise. The params.mongo options are now applied to both mongo methods. The _findOrGet method was also removed because it was no longer used.

Some other added features and perf gains to consider

• The pipeline can be used on all methods
• Multi patch and remove now support $limit and $sort
• Better performance for patch, update, and remove. This is accomplished by using the corresponding mongo methods that mutate and return, like findOneAndUpdate. This alleviates the need to re-lookup the record after mutation.
• Better performance for un-paginated find by not doing an unneeded countDocuments. And a couple other little tweaks around $limit: 0.
• Removed usage of Feathers adapter commons select function in favor of Mongo's built in projection.

I am also terrible at TS. So there may be some tweaks that need to be made there.

I meant that the MongoDB tests are not passing in CI: https://github.com/feathersjs/feathers/actions/runs/8850748298/job/24305731299?pr=3366#step:8:1588

Is there a way to make this more backwards compatible? Otherwise this'd have to wait for v6 - or we start moving things out again already which we were thinking of doing anyway.

Oh that's odd. I had just been running only the adapter tests locally and they worked. I hadn't noticed them failing in the CI suite. I can poke around at that later.

As far as backwards compatibility, there are three things I think.

The _findOrGet method could easily be added back. I actually just removed it today as I noticed it was no longer needed.

Using $limit and $sort in patch and remove could be easily removed. I am not sure if those are breaking changes or features. I lean feature.

For the params.pipeline and params.mongo switch, I don't think theres is a way to make it backwards compatible or that we want to. Being able to pass params.mongo to the aggregation function was actually a design goal of the changes. I should have made that more clear in previous posts. Previously you couldn't pass params.mongo to Model.aggregation because it was the "switch" and therefore called Model.find. Having options like collation, etc are valuable to both methods. And the "switch" was already a bit clunky because you then also used params.pipeline to customize aggregation. So the usage is now much clearer.

Previously, using Model.aggregate was the default. You had to pass params.mongo to use Model.find.

1 - If a user does not use params.mongo or params.pipeline. Previously it would have used Model.aggregate, now it uses Model.find. But the same results are returned, so not really a breaking change.
2 - If the user uses params.mongo. Previously it used Model.find, now it still uses Model.Find
3 - If the user uses params.pipeline. Previously it would have used Model.aggregate and it still uses Model.aggregate.
4 - If the user uses both params. Previously it would have used Model.find and ignored the params.pipeline, which is unexpected. Because they also used params.mongo it switched to find. Now it uses the appropriate Model.aggregate but also uses the params.mongo options.

So 1 works differently but shouldn't break anything. And 4 works differently, but previously it didn't work as expected.

So maybe it's not a breaking change?

I finally got around to fixing the broken test and added a few more. I think this is ready for @marshallswain to look at.

Excellent, thank you for putting this together!